[Style] Add custom scrollbar styling for SelectBox components #5879
Conversation
dosubot
bot
added
the
size:M
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 10/12/2025, 05:29:21 AM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Test Results❌ Some tests failed ⏰ Completed at: 10/12/2025, 05:45:13 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
christian-byrne
added
the
claude-review
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Comprehensive PR Review
This review is generated by Claude. It may not always be accurate, as with human reviewers. If you believe that any of the comments are invalid or incorrect, please state why for each. For others, please implement the changes in one way or another.
Review Summary
PR: [Style] Add custom scrollbar styling for SelectBox components (#5879)
Impact: 76 additions, 2 deletions across 3 files
Issue Distribution
- Critical: 0
- High: 1
- Medium: 4
- Low: 2
Category Breakdown
- Architecture: 3 issues
- Code Quality: 3 issues
- Performance: 1 issue
Key Findings
Architecture & Design
The implementation follows a reasonable approach for cross-browser scrollbar styling, but has some architectural inconsistencies. The PR introduces custom CSS components rather than following the existing Tailwind utility pattern used by scrollbar-hide. Additionally, the migration is incomplete - 8 other files still use scrollbar-hide, creating inconsistency across the application.
Code Quality Considerations
The main quality concern is the use of an undefined CSS color token (--color-white) which will cause fallbacks to browser defaults. The scrollbar hover behavior is also inconsistent in light theme where hover and normal states use the same color.
Performance Impact
The CSS includes scrollbar-gutter: stable both-edges which could cause layout shifts on browsers with inconsistent support. However, this is marked as low priority since it's ignored on unsupported browsers.
Integration Points
The Vue component changes are clean and maintain proper separation of concerns. The class replacement from scrollbar-hide to custom-scrollbar follows Vue best practices.
Positive Observations
- Comprehensive cross-browser support with both WebKit and Firefox implementations
- Well-documented CSS with clear usage instructions
- Clean Vue component implementation with proper class replacement
- Good use of CSS custom properties for theming
- Responsive design considerations for dark theme support
References
- Repository Architecture Guide
- ComfyUI Frontend Guidelines (CLAUDE.md) - emphasizes Tailwind utility-first approach
Next Steps
- Address the undefined color token issue (high priority)
- Consider completing the scrollbar migration across all components
- Evaluate whether to implement as Tailwind utility for consistency
- Test scrollbar behavior across target browsers
- Fix hover state inconsistency in light theme
This is a comprehensive automated review. The scrollbar implementation provides good user experience improvements, but addressing the color token and architectural consistency issues will improve maintainability.
viva-jinyi
force-pushed
the
style/custom-scrollbar
branch
from
6778d43 to
b7316ae
Compare
dosubot
bot
added
size:L
viva-jinyi
force-pushed
the
style/custom-scrollbar
branch
from
b7316ae to
ffb8d1b
Compare
|
I wanted to make all browsers display an identical scrollbar style controlled entirely by CSS, but it turns out Safari doesn’t allow a fully consistent implementation. To achieve pixel-perfect consistency across browsers, the only real option would be to hide the native scrollbar and build a custom overlay scrollbar (JS-based). What do you all think? |
dosubot
bot
added
size:M
|
@viva-jinyi Have you done custom scrollbar before? I have found it to be quite hard when not using a library, as there are many edge cases and relying on layout is hard. If we can't reach a satisfying solution here, should we try the other options from previous discussions?
|
viva-jinyi
force-pushed
the
style/custom-scrollbar
branch
from
2e90106 to
6284260
Compare
3 participants
Summary
Added custom scrollbar styling for SelectBox components (SingleSelect and MultiSelect) to improve visual consistency across the application.
Changes
.custom-scrollbarclass to SingleSelect and MultiSelect componentsImplementation Details
Browser Support
Screenshots
The custom scrollbar provides a more modern and consistent look across the application, especially in dropdown menus.
Testing
Before
After
┆Issue is synchronized with this Notion page by Unito